Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Editorial: widen parameter type of IsCompatiblePropertyDescriptor #3151

Merged
merged 1 commit into from
Aug 23, 2023

Conversation

doehyunbaek
Copy link
Contributor

Among three call sites of IsCompatiblePropertyDescriptor, one in [[GetOwnProperty]] internal method of a Proxy exotic object(step 14 with respect to es2023) it is possible that argument targetDesc is undefined. For example, in the below javascript code,

const monster1 = {
    eyeCount: 4,
};
const handler1 = {
    getOwnPropertyDescriptor(target, prop) {
        return { enumerable: true, configurable: true, value: "I'm a descriptor!" }
    },
};

const proxy1 = new Proxy(monster1, handler1);
proxy1.hasOwnProperty("noseCount"); // true

targetDesc is undefined as monster1 does not have noseCount property.

@ljharb ljharb changed the title Editorial: widen paramter type of IsCompatiblePropertyDescriptor Editorial: widen parameter type of IsCompatiblePropertyDescriptor Aug 20, 2023
Copy link
Contributor

@syg syg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch.

@michaelficarra michaelficarra added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Aug 23, 2023
…9#3151)

Among three call sites of IsCompatiblePropertyDescriptor, one in
[[GetOwnProperty]] internal method of a Proxy exotic object(step 14 with respect to es2023)
it is possible that argument `targetDesc` is *undefined*. For example,
in the below javascript code,

```javascript
const monster1 = {
    eyeCount: 4,
};
const handler1 = {
    getOwnPropertyDescriptor(target, prop) {
        return { enumerable: true, configurable: true, value: "I'm a descriptor!" }
    },
};

const proxy1 = new Proxy(monster1, handler1);
proxy1.hasOwnProperty("noseCount"); // true
```

`targetDesc` is undefined as `monster1` does not have `noseCount` property.
@ljharb ljharb force-pushed the widen_IsCompatiblePropertyDescriptor branch from a3c885b to 9d1f407 Compare August 23, 2023 21:52
@ljharb ljharb merged commit 9d1f407 into tc39:main Aug 23, 2023
6 checks passed
zhangenming pushed a commit to zhangenming/ecma262 that referenced this pull request Dec 22, 2023
…9#3151)

Among three call sites of IsCompatiblePropertyDescriptor, one in
[[GetOwnProperty]] internal method of a Proxy exotic object(step 14 with respect to es2023)
it is possible that argument `targetDesc` is *undefined*. For example,
in the below javascript code,

```javascript
const monster1 = {
    eyeCount: 4,
};
const handler1 = {
    getOwnPropertyDescriptor(target, prop) {
        return { enumerable: true, configurable: true, value: "I'm a descriptor!" }
    },
};

const proxy1 = new Proxy(monster1, handler1);
proxy1.hasOwnProperty("noseCount"); // true
```

`targetDesc` is undefined as `monster1` does not have `noseCount` property.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial change ready to merge Editors believe this PR needs no further reviews, and is ready to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants